From 23a0490b788619ec6476f110dc6619e8d6317d38 Mon Sep 17 00:00:00 2001 From: Aaron Date: Wed, 11 Apr 2012 10:51:02 -0700 Subject: [PATCH] [FileBackend] Added support for concurrent file write operations. * FS backends work via popen() and basic shell commands. * Swift backends use the custom SwiftCloudFiles async features. * Refactored storagePathsRead()/storagePathsChanged() to normalize the paths for correct dependency detection in FileOpBatch. * Cleaned up SwiftFileBackend exception handling to make debugging easier. * Added a quick and dirty performance testing script. * Updated unit tests to include a run with parallelize=implicit. * Improved file test failure output a bit. Change-Id: I6a5ed743c30c598e0dd7301dbdb3631c460332fd --- includes/AutoLoader.php | 4 + includes/filerepo/backend/FSFileBackend.php | 227 +++++++++-- includes/filerepo/backend/FileBackend.php | 23 ++ .../backend/FileBackendMultiWrite.php | 2 +- .../filerepo/backend/FileBackendStore.php | 85 +++- includes/filerepo/backend/FileOp.php | 319 +++++++-------- includes/filerepo/backend/FileOpBatch.php | 223 +++++++++++ .../filerepo/backend/SwiftFileBackend.php | 379 ++++++++++++------ maintenance/fileOpPerfTest.php | 135 +++++++ .../includes/filerepo/FileBackendTest.php | 159 ++++++-- 10 files changed, 1203 insertions(+), 353 deletions(-) create mode 100644 includes/filerepo/backend/FileOpBatch.php create mode 100644 maintenance/fileOpPerfTest.php diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 6710796a19..062fe8fbe5 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -514,14 +514,17 @@ $wgAutoloadLocalClasses = array( 'FileBackendStoreShardDirIterator' => 'includes/filerepo/backend/FileBackendStore.php', 'FileBackendStoreShardFileIterator' => 'includes/filerepo/backend/FileBackendStore.php', 'FileBackendMultiWrite' => 'includes/filerepo/backend/FileBackendMultiWrite.php', + 'FileBackendStoreOpHandle' => 'includes/filerepo/backend/FileBackendStore.php', 'FSFileBackend' => 'includes/filerepo/backend/FSFileBackend.php', 'FSFileBackendList' => 'includes/filerepo/backend/FSFileBackend.php', 'FSFileBackendDirList' => 'includes/filerepo/backend/FSFileBackend.php', 'FSFileBackendFileList' => 'includes/filerepo/backend/FSFileBackend.php', + 'FSFileOpHandle' => 'includes/filerepo/backend/FSFileBackend.php', 'SwiftFileBackend' => 'includes/filerepo/backend/SwiftFileBackend.php', 'SwiftFileBackendList' => 'includes/filerepo/backend/SwiftFileBackend.php', 'SwiftFileBackendDirList' => 'includes/filerepo/backend/SwiftFileBackend.php', 'SwiftFileBackendFileList' => 'includes/filerepo/backend/SwiftFileBackend.php', + 'SwiftFileOpHandle' => 'includes/filerepo/backend/SwiftFileBackend.php', 'FileJournal' => 'includes/filerepo/backend/filejournal/FileJournal.php', 'DBFileJournal' => 'includes/filerepo/backend/filejournal/DBFileJournal.php', 'NullFileJournal' => 'includes/filerepo/backend/filejournal/FileJournal.php', @@ -534,6 +537,7 @@ $wgAutoloadLocalClasses = array( 'MySqlLockManager'=> 'includes/filerepo/backend/lockmanager/DBLockManager.php', 'NullLockManager' => 'includes/filerepo/backend/lockmanager/LockManager.php', 'FileOp' => 'includes/filerepo/backend/FileOp.php', + 'FileOpBatch' => 'includes/filerepo/backend/FileOpBatch.php', 'StoreFileOp' => 'includes/filerepo/backend/FileOp.php', 'CopyFileOp' => 'includes/filerepo/backend/FileOp.php', 'MoveFileOp' => 'includes/filerepo/backend/FileOp.php', diff --git a/includes/filerepo/backend/FSFileBackend.php b/includes/filerepo/backend/FSFileBackend.php index d9a9c5945d..e1057306c4 100644 --- a/includes/filerepo/backend/FSFileBackend.php +++ b/includes/filerepo/backend/FSFileBackend.php @@ -191,22 +191,39 @@ class FSFileBackend extends FileBackendStore { } } - $ok = copy( $params['src'], $dest ); - // In some cases (at least over NFS), copy() returns true when it fails. - if ( !$ok || ( filesize( $params['src'] ) !== filesize( $dest ) ) ) { - if ( $ok ) { // PHP bug - unlink( $dest ); // remove broken file - trigger_error( __METHOD__ . ": copy() failed but returned true." ); + if ( !empty( $params['async'] ) ) { // deferred + $cmd = implode( ' ', array( wfIsWindows() ? 'COPY' : 'cp', + wfEscapeShellArg( $this->cleanPathSlashes( $params['src'] ) ), + wfEscapeShellArg( $this->cleanPathSlashes( $dest ) ) + ) ); + $status->value = new FSFileOpHandle( $this, $params, 'Store', $cmd, $dest ); + } else { // immediate write + $ok = copy( $params['src'], $dest ); + // In some cases (at least over NFS), copy() returns true when it fails + if ( !$ok || ( filesize( $params['src'] ) !== filesize( $dest ) ) ) { + if ( $ok ) { // PHP bug + unlink( $dest ); // remove broken file + trigger_error( __METHOD__ . ": copy() failed but returned true." ); + } + $status->fatal( 'backend-fail-store', $params['src'], $params['dst'] ); + return $status; } - $status->fatal( 'backend-fail-store', $params['src'], $params['dst'] ); - return $status; + $this->chmod( $dest ); } - $this->chmod( $dest ); - return $status; } + /** + * @see FSFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseStore( $errors, Status $status, array $params, $cmd ) { + if ( $errors !== '' && !( wfIsWindows() && $errors[0] === " " ) ) { + $status->fatal( 'backend-fail-store', $params['src'], $params['dst'] ); + trigger_error( "$cmd\n$errors", E_USER_WARNING ); // command output + } + } + /** * @see FileBackendStore::doCopyInternal() * @return Status @@ -239,22 +256,39 @@ class FSFileBackend extends FileBackendStore { } } - $ok = copy( $source, $dest ); - // In some cases (at least over NFS), copy() returns true when it fails. - if ( !$ok || ( filesize( $source ) !== filesize( $dest ) ) ) { - if ( $ok ) { // PHP bug - unlink( $dest ); // remove broken file - trigger_error( __METHOD__ . ": copy() failed but returned true." ); + if ( !empty( $params['async'] ) ) { // deferred + $cmd = implode( ' ', array( wfIsWindows() ? 'COPY' : 'cp', + wfEscapeShellArg( $this->cleanPathSlashes( $source ) ), + wfEscapeShellArg( $this->cleanPathSlashes( $dest ) ) + ) ); + $status->value = new FSFileOpHandle( $this, $params, 'Copy', $cmd, $dest ); + } else { // immediate write + $ok = copy( $source, $dest ); + // In some cases (at least over NFS), copy() returns true when it fails + if ( !$ok || ( filesize( $source ) !== filesize( $dest ) ) ) { + if ( $ok ) { // PHP bug + unlink( $dest ); // remove broken file + trigger_error( __METHOD__ . ": copy() failed but returned true." ); + } + $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); + return $status; } - $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); - return $status; + $this->chmod( $dest ); } - $this->chmod( $dest ); - return $status; } + /** + * @see FSFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseCopy( $errors, Status $status, array $params, $cmd ) { + if ( $errors !== '' && !( wfIsWindows() && $errors[0] === " " ) ) { + $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); + trigger_error( "$cmd\n$errors", E_USER_WARNING ); // command output + } + } + /** * @see FileBackendStore::doMoveInternal() * @return Status @@ -290,16 +324,34 @@ class FSFileBackend extends FileBackendStore { } } - $ok = rename( $source, $dest ); - clearstatcache(); // file no longer at source - if ( !$ok ) { - $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] ); - return $status; + if ( !empty( $params['async'] ) ) { // deferred + $cmd = implode( ' ', array( wfIsWindows() ? 'MOVE' : 'mv', + wfEscapeShellArg( $this->cleanPathSlashes( $source ) ), + wfEscapeShellArg( $this->cleanPathSlashes( $dest ) ) + ) ); + $status->value = new FSFileOpHandle( $this, $params, 'Move', $cmd ); + } else { // immediate write + $ok = rename( $source, $dest ); + clearstatcache(); // file no longer at source + if ( !$ok ) { + $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] ); + return $status; + } } return $status; } + /** + * @see FSFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseMove( $errors, Status $status, array $params, $cmd ) { + if ( $errors !== '' && !( wfIsWindows() && $errors[0] === " " ) ) { + $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] ); + trigger_error( "$cmd\n$errors", E_USER_WARNING ); // command output + } + } + /** * @see FileBackendStore::doDeleteInternal() * @return Status @@ -320,15 +372,32 @@ class FSFileBackend extends FileBackendStore { return $status; // do nothing; either OK or bad status } - $ok = unlink( $source ); - if ( !$ok ) { - $status->fatal( 'backend-fail-delete', $params['src'] ); - return $status; + if ( !empty( $params['async'] ) ) { // deferred + $cmd = implode( ' ', array( wfIsWindows() ? 'DEL' : 'unlink', + wfEscapeShellArg( $this->cleanPathSlashes( $source ) ) + ) ); + $status->value = new FSFileOpHandle( $this, $params, 'Copy', $cmd ); + } else { // immediate write + $ok = unlink( $source ); + if ( !$ok ) { + $status->fatal( 'backend-fail-delete', $params['src'] ); + return $status; + } } return $status; } + /** + * @see FSFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseDelete( $errors, Status $status, array $params, $cmd ) { + if ( $errors !== '' && !( wfIsWindows() && $errors[0] === " " ) ) { + $status->fatal( 'backend-fail-delete', $params['src'] ); + trigger_error( "$cmd\n$errors", E_USER_WARNING ); // command output + } + } + /** * @see FileBackendStore::doCreateInternal() * @return Status @@ -355,17 +424,45 @@ class FSFileBackend extends FileBackendStore { } } - $bytes = file_put_contents( $dest, $params['content'] ); - if ( $bytes === false ) { - $status->fatal( 'backend-fail-create', $params['dst'] ); - return $status; + if ( !empty( $params['async'] ) ) { // deferred + $tempFile = TempFSFile::factory( 'create_', 'tmp' ); + if ( !$tempFile ) { + $status->fatal( 'backend-fail-create', $params['dst'] ); + return $status; + } + $bytes = file_put_contents( $tempFile->getPath(), $params['content'] ); + if ( $bytes === false ) { + $status->fatal( 'backend-fail-create', $params['dst'] ); + return $status; + } + $cmd = implode( ' ', array( wfIsWindows() ? 'COPY' : 'cp', + wfEscapeShellArg( $this->cleanPathSlashes( $tempFile->getPath() ) ), + wfEscapeShellArg( $this->cleanPathSlashes( $dest ) ) + ) ); + $status->value = new FSFileOpHandle( $this, $params, 'Create', $cmd, $dest ); + $tempFile->bind( $status->value ); + } else { // immediate write + $bytes = file_put_contents( $dest, $params['content'] ); + if ( $bytes === false ) { + $status->fatal( 'backend-fail-create', $params['dst'] ); + return $status; + } + $this->chmod( $dest ); } - $this->chmod( $dest ); - return $status; } + /** + * @see FSFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseCreate( $errors, Status $status, array $params, $cmd ) { + if ( $errors !== '' && !( wfIsWindows() && $errors[0] === " " ) ) { + $status->fatal( 'backend-fail-create', $params['dst'] ); + trigger_error( "$cmd\n$errors", E_USER_WARNING ); // command output + } + } + /** * @see FileBackendStore::doPrepareInternal() * @return Status @@ -569,6 +666,40 @@ class FSFileBackend extends FileBackendStore { return false; } + /** + * @see FileBackendStore::doExecuteOpHandlesInternal() + * @return Array List of corresponding Status objects + */ + protected function doExecuteOpHandlesInternal( array $fileOpHandles ) { + $statuses = array(); + + $pipes = array(); + foreach ( $fileOpHandles as $index => $fileOpHandle ) { + $pipes[$index] = popen( "{$fileOpHandle->cmd} 2>&1", 'r' ); + } + + $errs = array(); + foreach ( $pipes as $index => $pipe ) { + // Result will be empty on success in *NIX. On Windows, + // it may be something like " 1 file(s) [copied|moved].". + $errs[$index] = stream_get_contents( $pipe ); + fclose( $pipe ); + } + + foreach ( $fileOpHandles as $index => $fileOpHandle ) { + $status = Status::newGood(); + $function = '_getResponse' . $fileOpHandle->call; + $this->$function( $errs[$index], $status, $fileOpHandle->params, $fileOpHandle->cmd ); + $statuses[$index] = $status; + if ( $status->isOK() && $fileOpHandle->chmodPath ) { + $this->chmod( $fileOpHandle->chmodPath ); + } + } + + clearstatcache(); // files changed + return $statuses; + } + /** * Chmod a file, suppressing the warnings * @@ -583,6 +714,16 @@ class FSFileBackend extends FileBackendStore { return $ok; } + /** + * Clean up directory separators for the given OS + * + * @param $path string FS path + * @return string + */ + protected function cleanPathSlashes( $path ) { + return wfIsWindows() ? strtr( $path, '/', '\\' ) : $path; + } + /** * Listen for E_WARNING errors and track whether any happen * @@ -610,6 +751,22 @@ class FSFileBackend extends FileBackendStore { } } +/** + * @see FileBackendStoreOpHandle + */ +class FSFileOpHandle extends FileBackendStoreOpHandle { + public $cmd; // string; shell command + public $chmodPath; // string; file to chmod + + public function __construct( $backend, array $params, $call, $cmd, $chmodPath = null ) { + $this->backend = $backend; + $this->params = $params; + $this->call = $call; + $this->cmd = $cmd; + $this->chmodPath = $chmodPath; + } +} + /** * Wrapper around RecursiveDirectoryIterator/DirectoryIterator that * catches exception or does any custom behavoir that we may want. diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php index 6f2d291647..24c68495ba 100644 --- a/includes/filerepo/backend/FileBackend.php +++ b/includes/filerepo/backend/FileBackend.php @@ -60,6 +60,9 @@ abstract class FileBackend { protected $name; // string; unique backend name protected $wikiId; // string; unique wiki name protected $readOnly; // string; read-only explanation message + protected $parallelize; // string; when to do operations in parallel + protected $concurrency; // integer; how many operations can be done in parallel + /** @var LockManager */ protected $lockManager; /** @var FileJournal */ @@ -80,6 +83,9 @@ abstract class FileBackend { * Journals simply log changes to files stored in the backend. * 'readOnly' : Write operations are disallowed if this is a non-empty string. * It should be an explanation for the backend being read-only. + * 'parallelize' : When to do file operations in parallel (when possible). + * Allowed values are "implicit", "explicit" and "off". + * 'concurrency' : How many file operations can be done in parallel. * * @param $config Array */ @@ -100,6 +106,12 @@ abstract class FileBackend { $this->readOnly = isset( $config['readOnly'] ) ? (string)$config['readOnly'] : ''; + $this->parallelize = isset( $config['parallelize'] ) + ? (string)$config['parallelize'] + : 'off'; + $this->concurrency = isset( $config['concurrency'] ) + ? (int)$config['concurrency'] + : 50; } /** @@ -204,6 +216,7 @@ abstract class FileBackend { * This has no effect unless the 'force' flag is set. * 'nonJournaled' : Don't log this operation batch in the file journal. * This limits the ability of recovery scripts. + * 'parallelize' : Try to do operations in parallel when possible. * * Remarks on locking: * File system paths given to operations should refer to files that are @@ -229,6 +242,16 @@ abstract class FileBackend { unset( $opts['nonLocking'] ); unset( $opts['allowStale'] ); } + $opts['concurrency'] = 1; // off + if ( $this->parallelize === 'implicit' ) { + if ( !isset( $opts['parallelize'] ) || $opts['parallelize'] ) { + $opts['concurrency'] = $this->concurrency; + } + } elseif ( $this->parallelize === 'explicit' ) { + if ( !empty( $opts['parallelize'] ) ) { + $opts['concurrency'] = $this->concurrency; + } + } return $this->doOperationsInternal( $ops, $opts ); } diff --git a/includes/filerepo/backend/FileBackendMultiWrite.php b/includes/filerepo/backend/FileBackendMultiWrite.php index 769aef69b0..4f9f0d9eee 100644 --- a/includes/filerepo/backend/FileBackendMultiWrite.php +++ b/includes/filerepo/backend/FileBackendMultiWrite.php @@ -143,7 +143,7 @@ class FileBackendMultiWrite extends FileBackend { } // Actually attempt the operation batch... - $subStatus = FileOp::attemptBatch( $performOps, $opts, $this->fileJournal ); + $subStatus = FileOpBatch::attempt( $performOps, $opts, $this->fileJournal ); $success = array(); $failCount = 0; diff --git a/includes/filerepo/backend/FileBackendStore.php b/includes/filerepo/backend/FileBackendStore.php index 30a64e298a..201f40f0cd 100644 --- a/includes/filerepo/backend/FileBackendStore.php +++ b/includes/filerepo/backend/FileBackendStore.php @@ -90,6 +90,9 @@ abstract class FileBackendStore extends FileBackend { * content : the raw file contents * dst : destination storage path * overwrite : overwrite any file that exists at the destination + * async : Status will be returned immediately if supported. + * If the status is OK, then its value field will be + * set to a FileBackendStoreOpHandle object. * * @param $params Array * @return Status @@ -123,6 +126,9 @@ abstract class FileBackendStore extends FileBackend { * src : source path on disk * dst : destination storage path * overwrite : overwrite any file that exists at the destination + * async : Status will be returned immediately if supported. + * If the status is OK, then its value field will be + * set to a FileBackendStoreOpHandle object. * * @param $params Array * @return Status @@ -155,6 +161,9 @@ abstract class FileBackendStore extends FileBackend { * src : source storage path * dst : destination storage path * overwrite : overwrite any file that exists at the destination + * async : Status will be returned immediately if supported. + * If the status is OK, then its value field will be + * set to a FileBackendStoreOpHandle object. * * @param $params Array * @return Status @@ -182,6 +191,9 @@ abstract class FileBackendStore extends FileBackend { * $params include: * src : source storage path * ignoreMissingSource : do nothing if the source file does not exist + * async : Status will be returned immediately if supported. + * If the status is OK, then its value field will be + * set to a FileBackendStoreOpHandle object. * * @param $params Array * @return Status @@ -210,6 +222,9 @@ abstract class FileBackendStore extends FileBackend { * src : source storage path * dst : destination storage path * overwrite : overwrite any file that exists at the destination + * async : Status will be returned immediately if supported. + * If the status is OK, then its value field will be + * set to a FileBackendStoreOpHandle object. * * @param $params Array * @return Status @@ -231,6 +246,7 @@ abstract class FileBackendStore extends FileBackend { * @return Status */ protected function doMoveInternal( array $params ) { + unset( $params['async'] ); // two steps, won't work here :) // Copy source to dest $status = $this->copyInternal( $params ); if ( $status->isOK() ) { @@ -907,7 +923,7 @@ abstract class FileBackendStore extends FileBackend { $this->primeContainerCache( $performOps ); // Actually attempt the operation batch... - $subStatus = FileOp::attemptBatch( $performOps, $opts, $this->fileJournal ); + $subStatus = FileOpBatch::attempt( $performOps, $opts, $this->fileJournal ); // Merge errors into status fields $status->merge( $subStatus ); @@ -918,6 +934,41 @@ abstract class FileBackendStore extends FileBackend { return $status; } + /** + * Execute a list of FileBackendStoreOpHandle handles in parallel. + * The resulting Status object fields will correspond + * to the order in which the handles where given. + * + * @param $handles Array List of FileBackendStoreOpHandle objects + * @return Array Map of Status objects + */ + final public function executeOpHandlesInternal( array $fileOpHandles ) { + wfProfileIn( __METHOD__ ); + wfProfileIn( __METHOD__ . '-' . $this->name ); + foreach ( $fileOpHandles as $fileOpHandle ) { + if ( !( $fileOpHandle instanceof FileBackendStoreOpHandle ) ) { + throw new MWException( "Given a non-FileBackendStoreOpHandle object." ); + } elseif ( $fileOpHandle->backend->getName() !== $this->getName() ) { + throw new MWException( "Given a FileBackendStoreOpHandle for the wrong backend." ); + } + } + $res = $this->doExecuteOpHandlesInternal( $fileOpHandles ); + wfProfileOut( __METHOD__ . '-' . $this->name ); + wfProfileOut( __METHOD__ ); + return $res; + } + + /** + * @see FileBackendStore::executeOpHandlesInternal() + * @return Array List of corresponding Status objects + */ + protected function doExecuteOpHandlesInternal( array $fileOpHandles ) { + foreach ( $fileOpHandles as $fileOpHandle ) { // OK if empty + throw new MWException( "This backend supports no asynchronous operations." ); + } + return array(); + } + /** * @see FileBackend::clearCache() */ @@ -1255,6 +1306,7 @@ abstract class FileBackendStore extends FileBackend { final protected function primeContainerCache( array $items ) { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); + $paths = array(); // list of storage paths $contNames = array(); // (cache key => resolved container name) // Get all the paths/containers from the items... @@ -1285,6 +1337,7 @@ abstract class FileBackendStore extends FileBackend { // Populate the container process cache for the backend... $this->doPrimeContainerCache( array_filter( $contInfo, 'is_array' ) ); + wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); } @@ -1345,6 +1398,7 @@ abstract class FileBackendStore extends FileBackend { final protected function primeFileCache( array $items ) { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); + $paths = array(); // list of storage paths $pathNames = array(); // (cache key => storage path) // Get all the paths/containers from the items... @@ -1371,11 +1425,40 @@ abstract class FileBackendStore extends FileBackend { $this->cache[$pathNames[$cacheKey]]['stat'] = $val; } } + wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); } } +/** + * FileBackendStore helper class for performing asynchronous file operations. + * + * For example, calling FileBackendStore::createInternal() with the "async" + * param flag may result in a Status that contains this object as a value. + * This class is largely backend-specific and is mostly just "magic" to be + * passed to FileBackendStore::executeOpHandlesInternal(). + */ +abstract class FileBackendStoreOpHandle { + /** @var Array */ + public $params = array(); // params to caller functions + /** @var FileBackendStore */ + public $backend; + /** @var Array */ + public $resourcesToClose = array(); + + public $call; // string; name that identifies the function called + + /** + * Close all open file handles + * + * @return void + */ + public function closeResources() { + array_map( 'fclose', $this->resourcesToClose ); + } +} + /** * FileBackendStore helper function to handle listings that span container shards. * Do not use this class from places outside of FileBackendStore. diff --git a/includes/filerepo/backend/FileOp.php b/includes/filerepo/backend/FileOp.php index 8a2d4285aa..b0ba6c2103 100644 --- a/includes/filerepo/backend/FileOp.php +++ b/includes/filerepo/backend/FileOp.php @@ -23,11 +23,12 @@ */ /** - * Helper class for representing operations with transaction support. - * Do not use this class from places outside FileBackend. + * FileBackend helper class for representing operations. + * Do naot use this class from places outside FileBackend. * - * Methods called from attemptBatch() should avoid throwing exceptions at all costs. - * FileOp objects should be lightweight in order to support large arrays in memory. + * Methods called from FileOpBatch::attempt() should avoid throwing + * exceptions at all costs. FileOp objects should be lightweight in order + * to support large arrays in memory and serialization. * * @ingroup FileBackend * @since 1.19 @@ -40,6 +41,7 @@ abstract class FileOp { protected $state = self::STATE_NEW; // integer protected $failed = false; // boolean + protected $async = false; // boolean protected $useLatest = true; // boolean protected $batchId; // string @@ -51,10 +53,6 @@ abstract class FileOp { const STATE_CHECKED = 2; const STATE_ATTEMPTED = 3; - /* Timeout related parameters */ - const MAX_BATCH_SIZE = 1000; - const TIME_LIMIT_SEC = 300; // 5 minutes - /** * Build a new file operation transaction * @@ -86,7 +84,7 @@ abstract class FileOp { * @param $batchId string * @return void */ - final protected function setBatchId( $batchId ) { + final public function setBatchId( $batchId ) { $this->batchId = $batchId; } @@ -96,130 +94,99 @@ abstract class FileOp { * @param $allowStale bool * @return void */ - final protected function allowStaleReads( $allowStale ) { + final public function allowStaleReads( $allowStale ) { $this->useLatest = !$allowStale; } /** - * Attempt to perform a series of file operations. - * Callers are responsible for handling file locking. + * Get the value of the parameter with the given name * - * $opts is an array of options, including: - * 'force' : Errors that would normally cause a rollback do not. - * The remaining operations are still attempted if any fail. - * 'allowStale' : Don't require the latest available data. - * This can increase performance for non-critical writes. - * This has no effect unless the 'force' flag is set. - * 'nonJournaled' : Don't log this operation batch in the file journal. + * @param $name string + * @return mixed Returns null if the parameter is not set + */ + final public function getParam( $name ) { + return isset( $this->params[$name] ) ? $this->params[$name] : null; + } + + /** + * Check if this operation failed precheck() or attempt() * - * The resulting Status will be "OK" unless: - * a) unexpected operation errors occurred (network partitions, disk full...) - * b) significant operation errors occured and 'force' was not set + * @return bool + */ + final public function failed() { + return $this->failed; + } + + /** + * Get a new empty predicates array for precheck() * - * @param $performOps Array List of FileOp operations - * @param $opts Array Batch operation options - * @param $journal FileJournal Journal to log operations to - * @return Status + * @return Array */ - final public static function attemptBatch( - array $performOps, array $opts, FileJournal $journal - ) { - $status = Status::newGood(); + final public static function newPredicates() { + return array( 'exists' => array(), 'sha1' => array() ); + } - $n = count( $performOps ); - if ( $n > self::MAX_BATCH_SIZE ) { - $status->fatal( 'backend-fail-batchsize', $n, self::MAX_BATCH_SIZE ); - return $status; - } + /** + * Get a new empty dependency tracking array for paths read/written to + * + * @return Array + */ + final public static function newDependencies() { + return array( 'read' => array(), 'write' => array() ); + } - $batchId = $journal->getTimestampedUUID(); - $allowStale = !empty( $opts['allowStale'] ); - $ignoreErrors = !empty( $opts['force'] ); - $journaled = empty( $opts['nonJournaled'] ); - - $entries = array(); // file journal entries - $predicates = FileOp::newPredicates(); // account for previous op in prechecks - // Do pre-checks for each operation; abort on failure... - foreach ( $performOps as $index => $fileOp ) { - $fileOp->setBatchId( $batchId ); - $fileOp->allowStaleReads( $allowStale ); - $oldPredicates = $predicates; - $subStatus = $fileOp->precheck( $predicates ); // updates $predicates - $status->merge( $subStatus ); - if ( $subStatus->isOK() ) { - if ( $journaled ) { // journal log entry - $entries = array_merge( $entries, - self::getJournalEntries( $fileOp, $oldPredicates, $predicates ) ); - } - } else { // operation failed? - $status->success[$index] = false; - ++$status->failCount; - if ( !$ignoreErrors ) { - return $status; // abort - } - } - } + /** + * Update a dependency tracking array to account for this operation + * + * @param $deps Array Prior path reads/writes; format of FileOp::newPredicates() + * @return Array + */ + final public function applyDependencies( array $deps ) { + $deps['read'] += array_fill_keys( $this->storagePathsRead(), 1 ); + $deps['write'] += array_fill_keys( $this->storagePathsChanged(), 1 ); + return $deps; + } - // Log the operations in file journal... - if ( count( $entries ) ) { - $subStatus = $journal->logChangeBatch( $entries, $batchId ); - if ( !$subStatus->isOK() ) { - return $subStatus; // abort + /** + * Check if this operation changes files listed in $paths + * + * @param $paths Array Prior path reads/writes; format of FileOp::newPredicates() + * @return boolean + */ + final public function dependsOn( array $deps ) { + foreach ( $this->storagePathsChanged() as $path ) { + if ( isset( $deps['read'][$path] ) || isset( $deps['write'][$path] ) ) { + return true; // "output" or "anti" dependency } } - - if ( $ignoreErrors ) { // treat precheck() fatals as mere warnings - $status->setResult( true, $status->value ); - } - - // Attempt each operation... - foreach ( $performOps as $index => $fileOp ) { - if ( $fileOp->failed() ) { - continue; // nothing to do - } - $subStatus = $fileOp->attempt(); - $status->merge( $subStatus ); - if ( $subStatus->isOK() ) { - $status->success[$index] = true; - ++$status->successCount; - } else { - $status->success[$index] = false; - ++$status->failCount; - // We can't continue (even with $ignoreErrors) as $predicates is wrong. - // Log the remaining ops as failed for recovery... - for ( $i = ($index + 1); $i < count( $performOps ); $i++ ) { - $performOps[$i]->logFailure( 'attempt_aborted' ); - } - return $status; // bail out + foreach ( $this->storagePathsRead() as $path ) { + if ( isset( $deps['write'][$path] ) ) { + return true; // "flow" dependency } } - - return $status; + return false; } /** - * Get the file journal entries for a single file operation + * Get the file journal entries for this file operation * - * @param $fileOp FileOp - * @param $oPredicates Array Pre-op information about files - * @param $nPredicates Array Post-op information about files + * @param $oPredicates Array Pre-op info about files (format of FileOp::newPredicates) + * @param $nPredicates Array Post-op info about files (format of FileOp::newPredicates) * @return Array */ - final protected static function getJournalEntries( - FileOp $fileOp, array $oPredicates, array $nPredicates - ) { + final public function getJournalEntries( array $oPredicates, array $nPredicates ) { $nullEntries = array(); $updateEntries = array(); $deleteEntries = array(); - $pathsUsed = array_merge( $fileOp->storagePathsRead(), $fileOp->storagePathsChanged() ); + $pathsUsed = array_merge( $this->storagePathsRead(), $this->storagePathsChanged() ); foreach ( $pathsUsed as $path ) { $nullEntries[] = array( // assertion for recovery 'op' => 'null', 'path' => $path, - 'newSha1' => $fileOp->fileSha1( $path, $oPredicates ) + 'newSha1' => $this->fileSha1( $path, $oPredicates ) ); } - foreach ( $fileOp->storagePathsChanged() as $path ) { + foreach ( $this->storagePathsChanged() as $path ) { if ( $nPredicates['sha1'][$path] === false ) { // deleted $deleteEntries[] = array( 'op' => 'delete', @@ -228,7 +195,7 @@ abstract class FileOp { ); } else { // created/updated $updateEntries[] = array( - 'op' => $fileOp->fileExists( $path, $oPredicates ) ? 'update' : 'create', + 'op' => $this->fileExists( $path, $oPredicates ) ? 'update' : 'create', 'path' => $path, 'newSha1' => $nPredicates['sha1'][$path] ); @@ -237,34 +204,6 @@ abstract class FileOp { return array_merge( $nullEntries, $updateEntries, $deleteEntries ); } - /** - * Get the value of the parameter with the given name - * - * @param $name string - * @return mixed Returns null if the parameter is not set - */ - final public function getParam( $name ) { - return isset( $this->params[$name] ) ? $this->params[$name] : null; - } - - /** - * Check if this operation failed precheck() or attempt() - * - * @return bool - */ - final public function failed() { - return $this->failed; - } - - /** - * Get a new empty predicates array for precheck() - * - * @return Array - */ - final public static function newPredicates() { - return array( 'exists' => array(), 'sha1' => array() ); - } - /** * Check preconditions of the operation without writing anything * @@ -284,7 +223,14 @@ abstract class FileOp { } /** - * Attempt the operation, backing up files as needed; this must be reversible + * @return Status + */ + protected function doPrecheck( array &$predicates ) { + return Status::newGood(); + } + + /** + * Attempt the operation * * @return Status */ @@ -303,6 +249,25 @@ abstract class FileOp { return $status; } + /** + * @return Status + */ + protected function doAttempt() { + return Status::newGood(); + } + + /** + * Attempt the operation in the background + * + * @return Status + */ + final public function attemptAsync() { + $this->async = true; + $result = $this->attempt(); + $this->async = false; + return $result; + } + /** * Get the file operation parameters * @@ -312,36 +277,48 @@ abstract class FileOp { return array( array(), array() ); } + /** + * Adjust params to FileBackendStore internal file calls + * + * @param $params Array + * @return Array (required params list, optional params list) + */ + protected function setFlags( array $params ) { + return array( 'async' => $this->async ) + $params; + } + /** * Get a list of storage paths read from for this operation * * @return Array */ - public function storagePathsRead() { - return array(); + final public function storagePathsRead() { + return array_map( 'FileBackend::normalizeStoragePath', $this->doStoragePathsRead() ); } /** - * Get a list of storage paths written to for this operation - * + * @see FileOp::storagePathsRead() * @return Array */ - public function storagePathsChanged() { + protected function doStoragePathsRead() { return array(); } /** - * @return Status + * Get a list of storage paths written to for this operation + * + * @return Array */ - protected function doPrecheck( array &$predicates ) { - return Status::newGood(); + final public function storagePathsChanged() { + return array_map( 'FileBackend::normalizeStoragePath', $this->doStoragePathsChanged() ); } /** - * @return Status + * @see FileOp::storagePathsChanged() + * @return Array */ - protected function doAttempt() { - return Status::newGood(); + protected function doStoragePathsChanged() { + return array(); } /** @@ -425,13 +402,22 @@ abstract class FileOp { } } + /** + * Get the backend this operation is for + * + * @return FileBackendStore + */ + public function getBackend() { + return $this->backend; + } + /** * Log a file operation failure and preserve any temp files * * @param $action string * @return void */ - final protected function logFailure( $action ) { + final public function logFailure( $action ) { $params = $this->params; $params['failedAction'] = $action; try { @@ -482,12 +468,11 @@ class StoreFileOp extends FileOp { } protected function doAttempt() { - $status = Status::newGood(); // Store the file at the destination if ( !$this->destSameAsSource ) { - $status->merge( $this->backend->storeInternal( $this->params ) ); + return $this->backend->storeInternal( $this->setFlags( $this->params ) ); } - return $status; + return Status::newGood(); } protected function getSourceSha1Base36() { @@ -500,7 +485,7 @@ class StoreFileOp extends FileOp { return $hash; } - public function storagePathsChanged() { + protected function doStoragePathsChanged() { return array( $this->params['dst'] ); } } @@ -540,19 +525,18 @@ class CreateFileOp extends FileOp { } protected function doAttempt() { - $status = Status::newGood(); - // Create the file at the destination if ( !$this->destSameAsSource ) { - $status->merge( $this->backend->createInternal( $this->params ) ); + // Create the file at the destination + return $this->backend->createInternal( $this->setFlags( $this->params ) ); } - return $status; + return Status::newGood(); } protected function getSourceSha1Base36() { return wfBaseConvert( sha1( $this->params['content'] ), 16, 36, 31 ); } - public function storagePathsChanged() { + protected function doStoragePathsChanged() { return array( $this->params['dst'] ); } } @@ -592,22 +576,21 @@ class CopyFileOp extends FileOp { } protected function doAttempt() { - $status = Status::newGood(); // Do nothing if the src/dst paths are the same if ( $this->params['src'] !== $this->params['dst'] ) { // Copy the file into the destination if ( !$this->destSameAsSource ) { - $status->merge( $this->backend->copyInternal( $this->params ) ); + return $this->backend->copyInternal( $this->setFlags( $this->params ) ); } } - return $status; + return Status::newGood(); } - public function storagePathsRead() { + protected function doStoragePathsRead() { return array( $this->params['src'] ); } - public function storagePathsChanged() { + protected function doStoragePathsChanged() { return array( $this->params['dst'] ); } } @@ -649,26 +632,25 @@ class MoveFileOp extends FileOp { } protected function doAttempt() { - $status = Status::newGood(); // Do nothing if the src/dst paths are the same if ( $this->params['src'] !== $this->params['dst'] ) { if ( !$this->destSameAsSource ) { // Move the file into the destination - $status->merge( $this->backend->moveInternal( $this->params ) ); + return $this->backend->moveInternal( $this->setFlags( $this->params ) ); } else { // Just delete source as the destination needs no changes $params = array( 'src' => $this->params['src'] ); - $status->merge( $this->backend->deleteInternal( $params ) ); + return $this->backend->deleteInternal( $this->setFlags( $params ) ); } } - return $status; + return Status::newGood(); } - public function storagePathsRead() { + protected function doStoragePathsRead() { return array( $this->params['src'] ); } - public function storagePathsChanged() { + protected function doStoragePathsChanged() { return array( $this->params['src'], $this->params['dst'] ); } } @@ -703,15 +685,14 @@ class DeleteFileOp extends FileOp { } protected function doAttempt() { - $status = Status::newGood(); if ( $this->needsDelete ) { // Delete the source file - $status->merge( $this->backend->deleteInternal( $this->params ) ); + return $this->backend->deleteInternal( $this->setFlags( $this->params ) ); } - return $status; + return Status::newGood(); } - public function storagePathsChanged() { + protected function doStoragePathsChanged() { return array( $this->params['src'] ); } } diff --git a/includes/filerepo/backend/FileOpBatch.php b/includes/filerepo/backend/FileOpBatch.php new file mode 100644 index 0000000000..049f2c54d9 --- /dev/null +++ b/includes/filerepo/backend/FileOpBatch.php @@ -0,0 +1,223 @@ + self::MAX_BATCH_SIZE ) { + $status->fatal( 'backend-fail-batchsize', $n, self::MAX_BATCH_SIZE ); + wfProfileOut( __METHOD__ ); + return $status; + } + + $batchId = $journal->getTimestampedUUID(); + $allowStale = !empty( $opts['allowStale'] ); + $ignoreErrors = !empty( $opts['force'] ); + $journaled = empty( $opts['nonJournaled'] ); + $maxConcurrency = isset( $opts['concurrency'] ) ? $opts['concurrency'] : 1; + + $entries = array(); // file journal entry list + $predicates = FileOp::newPredicates(); // account for previous ops in prechecks + $curBatch = array(); // concurrent FileOp sub-batch accumulation + $curBatchDeps = FileOp::newDependencies(); // paths used in FileOp sub-batch + $pPerformOps = array(); // ordered list of concurrent FileOp sub-batches + $lastBackend = null; // last op backend name + // Do pre-checks for each operation; abort on failure... + foreach ( $performOps as $index => $fileOp ) { + $backendName = $fileOp->getBackend()->getName(); + $fileOp->setBatchId( $batchId ); // transaction ID + $fileOp->allowStaleReads( $allowStale ); // consistency level + // Decide if this op can be done concurrently within this sub-batch + // or if a new concurrent sub-batch must be started after this one... + if ( $fileOp->dependsOn( $curBatchDeps ) + || count( $curBatch ) >= $maxConcurrency + || ( $backendName !== $lastBackend && count( $curBatch ) ) + ) { + $pPerformOps[] = $curBatch; // push this batch + $curBatch = array(); // start a new sub-batch + $curBatchDeps = FileOp::newDependencies(); + } + $lastBackend = $backendName; + $curBatch[$index] = $fileOp; // keep index + // Update list of affected paths in this batch + $curBatchDeps = $fileOp->applyDependencies( $curBatchDeps ); + // Simulate performing the operation... + $oldPredicates = $predicates; + $subStatus = $fileOp->precheck( $predicates ); // updates $predicates + $status->merge( $subStatus ); + if ( $subStatus->isOK() ) { + if ( $journaled ) { // journal log entries + $entries = array_merge( $entries, + $fileOp->getJournalEntries( $oldPredicates, $predicates ) ); + } + } else { // operation failed? + $status->success[$index] = false; + ++$status->failCount; + if ( !$ignoreErrors ) { + wfProfileOut( __METHOD__ ); + return $status; // abort + } + } + } + // Push the last sub-batch + if ( count( $curBatch ) ) { + $pPerformOps[] = $curBatch; + } + + // Log the operations in the file journal... + if ( count( $entries ) ) { + $subStatus = $journal->logChangeBatch( $entries, $batchId ); + if ( !$subStatus->isOK() ) { + wfProfileOut( __METHOD__ ); + return $subStatus; // abort + } + } + + if ( $ignoreErrors ) { // treat precheck() fatals as mere warnings + $status->setResult( true, $status->value ); + } + + // Attempt each operation (in parallel if allowed and possible)... + if ( count( $pPerformOps ) < count( $performOps ) ) { + self::runBatchParallel( $pPerformOps, $status ); + } else { + self::runBatchSeries( $performOps, $status ); + } + + wfProfileOut( __METHOD__ ); + return $status; + } + + /** + * Attempt a list of file operations in series. + * This will abort remaining ops on failure. + * + * @param $performOps Array + * @param $status Status + * @return bool Success + */ + protected static function runBatchSeries( array $performOps, Status $status ) { + foreach ( $performOps as $index => $fileOp ) { + if ( $fileOp->failed() ) { + continue; // nothing to do + } + $subStatus = $fileOp->attempt(); + $status->merge( $subStatus ); + if ( $subStatus->isOK() ) { + $status->success[$index] = true; + ++$status->successCount; + } else { + $status->success[$index] = false; + ++$status->failCount; + // We can't continue (even with $ignoreErrors) as $predicates is wrong. + // Log the remaining ops as failed for recovery... + for ( $i = ($index + 1); $i < count( $performOps ); $i++ ) { + $performOps[$i]->logFailure( 'attempt_aborted' ); + } + return false; // bail out + } + } + return true; + } + + /** + * Attempt a list of file operations sub-batches in series. + * + * The operations *in* each sub-batch will be done in parallel. + * The caller is responsible for making sure the operations + * within any given sub-batch do not depend on each other. + * This will abort remaining ops on failure. + * + * @param $performOps Array + * @param $status Status + * @return bool Success + */ + protected static function runBatchParallel( array $pPerformOps, Status $status ) { + $aborted = false; + foreach ( $pPerformOps as $performOpsBatch ) { + if ( $aborted ) { // check batch op abort flag... + // We can't continue (even with $ignoreErrors) as $predicates is wrong. + // Log the remaining ops as failed for recovery... + foreach ( $performOpsBatch as $i => $fileOp ) { + $performOpsBatch[$i]->logFailure( 'attempt_aborted' ); + } + continue; + } + $statuses = array(); + $opHandles = array(); + // Get the backend; all sub-batch ops belong to a single backend + $backend = reset( $performOpsBatch )->getBackend(); + // If attemptAsync() returns synchronously, it was either an + // error Status or the backend just doesn't support async ops. + foreach ( $performOpsBatch as $i => $fileOp ) { + if ( !$fileOp->failed() ) { // failed => already has Status + $subStatus = $fileOp->attemptAsync(); + if ( $subStatus->value instanceof FileBackendStoreOpHandle ) { + $opHandles[$i] = $subStatus->value; // deferred + } else { + $statuses[$i] = $subStatus; // done already + } + } + } + // Try to do all the operations concurrently... + $statuses = $statuses + $backend->executeOpHandlesInternal( $opHandles ); + // Marshall and merge all the responses (blocking)... + foreach ( $performOpsBatch as $i => $fileOp ) { + if ( !$fileOp->failed() ) { // failed => already has Status + $subStatus = $statuses[$i]; + $status->merge( $subStatus ); + if ( $subStatus->isOK() ) { + $status->success[$i] = true; + ++$status->successCount; + } else { + $status->success[$i] = false; + ++$status->failCount; + $aborted = true; // set abort flag; we can't continue + } + } + } + } + return $status; + } +} diff --git a/includes/filerepo/backend/SwiftFileBackend.php b/includes/filerepo/backend/SwiftFileBackend.php index f40c3236e4..0def61fc04 100644 --- a/includes/filerepo/backend/SwiftFileBackend.php +++ b/includes/filerepo/backend/SwiftFileBackend.php @@ -64,6 +64,9 @@ class SwiftFileBackend extends FileBackendStore { */ public function __construct( array $config ) { parent::__construct( $config ); + if ( !MWInit::classExists( 'CF_Constants' ) ) { + throw new MWException( 'SwiftCloudFiles extension not installed.' ); + } // Required settings $this->auth = new CF_Authentication( $config['swiftUser'], @@ -110,9 +113,8 @@ class SwiftFileBackend extends FileBackendStore { $this->getContainer( $container ); return true; // container exists } catch ( NoSuchContainerException $e ) { - } catch ( InvalidResponseException $e ) { - } catch ( Exception $e ) { // some other exception? - $this->logException( $e, __METHOD__, array( 'path' => $storagePath ) ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, null, __METHOD__, array( 'path' => $storagePath ) ); } return false; @@ -143,12 +145,8 @@ class SwiftFileBackend extends FileBackendStore { } catch ( NoSuchContainerException $e ) { $status->fatal( 'backend-fail-create', $params['dst'] ); return $status; - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } @@ -167,20 +165,32 @@ class SwiftFileBackend extends FileBackendStore { $obj->set_etag( md5( $params['content'] ) ); // Use the same content type as StreamFile for security $obj->content_type = StreamFile::contentTypeFromPath( $params['dst'] ); - // Actually write the object in Swift - $obj->write( $params['content'] ); + if ( !empty( $params['async'] ) ) { // deferred + $handle = $obj->write_async( $params['content'] ); + $status->value = new SwiftFileOpHandle( $this, $params, 'Create', $handle ); + } else { // actually write the object in Swift + $obj->write( $params['content'] ); + } } catch ( BadContentTypeException $e ) { $status->fatal( 'backend-fail-contenttype', $params['dst'] ); - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); } return $status; } + /** + * @see SwiftFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseCreate( CF_Async_Op $cfOp, Status $status, array $params ) { + try { + $cfOp->getLastResponse(); + } catch ( BadContentTypeException $e ) { + $status->fatal( 'backend-fail-contenttype', $params['dst'] ); + } + } + /** * @see FileBackendStore::doStoreInternal() * @return Status @@ -206,12 +216,8 @@ class SwiftFileBackend extends FileBackendStore { } catch ( NoSuchContainerException $e ) { $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); return $status; - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } @@ -234,22 +240,44 @@ class SwiftFileBackend extends FileBackendStore { $obj->set_etag( md5_file( $params['src'] ) ); // Use the same content type as StreamFile for security $obj->content_type = StreamFile::contentTypeFromPath( $params['dst'] ); - // Actually write the object in Swift - $obj->load_from_filename( $params['src'], True ); // calls $obj->write() + if ( !empty( $params['async'] ) ) { // deferred + wfSuppressWarnings(); + $fp = fopen( $params['src'], 'rb' ); + wfRestoreWarnings(); + if ( !$fp ) { + $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); + } else { + $handle = $obj->write_async( $fp, filesize( $params['src'] ), true ); + $status->value = new SwiftFileOpHandle( $this, $params, 'Store', $handle ); + $status->value->resourcesToClose[] = $fp; + } + } else { // actually write the object in Swift + $obj->load_from_filename( $params['src'], true ); // calls $obj->write() + } } catch ( BadContentTypeException $e ) { $status->fatal( 'backend-fail-contenttype', $params['dst'] ); } catch ( IOException $e ) { $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); } return $status; } + /** + * @see SwiftFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseStore( CF_Async_Op $cfOp, Status $status, array $params ) { + try { + $cfOp->getLastResponse(); + } catch ( BadContentTypeException $e ) { + $status->fatal( 'backend-fail-contenttype', $params['dst'] ); + } catch ( IOException $e ) { + $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); + } + } + /** * @see FileBackendStore::doCopyInternal() * @return Status @@ -282,30 +310,104 @@ class SwiftFileBackend extends FileBackendStore { } catch ( NoSuchContainerException $e ) { $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); return $status; - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } // (b) Actually copy the file to the destination try { - $sContObj->copy_object_to( $srcRel, $dContObj, $dstRel ); + if ( !empty( $params['async'] ) ) { // deferred + $handle = $sContObj->copy_object_to_async( $srcRel, $dContObj, $dstRel ); + $status->value = new SwiftFileOpHandle( $this, $params, 'Copy', $handle ); + } else { // actually write the object in Swift + $sContObj->copy_object_to( $srcRel, $dContObj, $dstRel ); + } + } catch ( NoSuchObjectException $e ) { // source object does not exist + $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); + } + + return $status; + } + + /** + * @see SwiftFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseCopy( CF_Async_Op $cfOp, Status $status, array $params ) { + try { + $cfOp->getLastResponse(); } catch ( NoSuchObjectException $e ) { // source object does not exist $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] ); - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } + } + + /** + * @see FileBackendStore::doMoveInternal() + * @return Status + */ + protected function doMoveInternal( array $params ) { + $status = Status::newGood(); + + list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] ); + if ( $srcRel === null ) { + $status->fatal( 'backend-fail-invalidpath', $params['src'] ); + return $status; + } + + list( $dstCont, $dstRel ) = $this->resolveStoragePathReal( $params['dst'] ); + if ( $dstRel === null ) { + $status->fatal( 'backend-fail-invalidpath', $params['dst'] ); + return $status; + } + + // (a) Check the source/destination containers and destination object + try { + $sContObj = $this->getContainer( $srcCont ); + $dContObj = $this->getContainer( $dstCont ); + if ( empty( $params['overwrite'] ) && + $this->fileExists( array( 'src' => $params['dst'], 'latest' => 1 ) ) ) + { + $status->fatal( 'backend-fail-alreadyexists', $params['dst'] ); + return $status; + } + } catch ( NoSuchContainerException $e ) { + $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] ); + return $status; + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); + return $status; + } + + // (b) Actually move the file to the destination + try { + if ( !empty( $params['async'] ) ) { // deferred + $handle = $sContObj->move_object_to_async( $srcRel, $dContObj, $dstRel ); + $status->value = new SwiftFileOpHandle( $this, $params, 'Move', $handle ); + } else { // actually write the object in Swift + $sContObj->move_object_to( $srcRel, $dContObj, $dstRel ); + } + } catch ( NoSuchObjectException $e ) { // source object does not exist + $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); } return $status; } + /** + * @see SwiftFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseMove( CF_Async_Op $cfOp, Status $status, array $params ) { + try { + $cfOp->getLastResponse(); + } catch ( NoSuchObjectException $e ) { // source object does not exist + $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] ); + } + } + /** * @see FileBackendStore::doDeleteInternal() * @return Status @@ -321,23 +423,40 @@ class SwiftFileBackend extends FileBackendStore { try { $sContObj = $this->getContainer( $srcCont ); - $sContObj->delete_object( $srcRel ); + if ( !empty( $params['async'] ) ) { // deferred + $handle = $sContObj->delete_object_async( $srcRel ); + $status->value = new SwiftFileOpHandle( $this, $params, 'Delete', $handle ); + } else { // actually write the object in Swift + $sContObj->delete_object( $srcRel ); + } } catch ( NoSuchContainerException $e ) { $status->fatal( 'backend-fail-delete', $params['src'] ); } catch ( NoSuchObjectException $e ) { if ( empty( $params['ignoreMissingSource'] ) ) { $status->fatal( 'backend-fail-delete', $params['src'] ); } - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); } return $status; } + /** + * @see SwiftFileBackend::doExecuteOpHandlesInternal() + */ + protected function _getResponseDelete( CF_Async_Op $cfOp, Status $status, array $params ) { + try { + $cfOp->getLastResponse(); + } catch ( NoSuchContainerException $e ) { + $status->fatal( 'backend-fail-delete', $params['src'] ); + } catch ( NoSuchObjectException $e ) { + if ( empty( $params['ignoreMissingSource'] ) ) { + $status->fatal( 'backend-fail-delete', $params['src'] ); + } + } + } + /** * @see FileBackendStore::doPrepareInternal() * @return Status @@ -352,12 +471,8 @@ class SwiftFileBackend extends FileBackendStore { return $status; // already exists } catch ( NoSuchContainerException $e ) { // NoSuchContainerException thrown: container does not exist - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } @@ -372,12 +487,8 @@ class SwiftFileBackend extends FileBackendStore { array( $this->auth->username ) // write ) ); } - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } @@ -408,11 +519,8 @@ class SwiftFileBackend extends FileBackendStore { // metadata, we can make use of that to avoid RTTs $contObj->mw_wasSecured = true; // avoid useless RTTs } - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); } } @@ -436,12 +544,8 @@ class SwiftFileBackend extends FileBackendStore { $contObj = $this->getContainer( $fullCont, true ); } catch ( NoSuchContainerException $e ) { return $status; // ok, nothing to do - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } @@ -451,12 +555,10 @@ class SwiftFileBackend extends FileBackendStore { $this->deleteContainer( $fullCont ); } catch ( NoSuchContainerException $e ) { return $status; // race? - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-internal', $this->name ); - $this->logException( $e, __METHOD__, $params ); + } catch ( NonEmptyContainerException $e ) { + return $status; // race? consistency delay? + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } } @@ -487,11 +589,9 @@ class SwiftFileBackend extends FileBackendStore { ); } catch ( NoSuchContainerException $e ) { } catch ( NoSuchObjectException $e ) { - } catch ( InvalidResponseException $e ) { + } catch ( CloudFilesException $e ) { // some other exception? $stat = null; - } catch ( Exception $e ) { // some other exception? - $stat = null; - $this->logException( $e, __METHOD__, $params ); + $this->handleException( $e, null, __METHOD__, $params ); } return $stat; @@ -546,9 +646,8 @@ class SwiftFileBackend extends FileBackendStore { $obj = new CF_Object( $sContObj, $srcRel, false, false ); // skip HEAD request $data = $obj->read( $this->headersFromParams( $params ) ); } catch ( NoSuchContainerException $e ) { - } catch ( InvalidResponseException $e ) { - } catch ( Exception $e ) { // some other exception? - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, null, __METHOD__, $params ); } return $data; @@ -565,9 +664,9 @@ class SwiftFileBackend extends FileBackendStore { return ( count( $container->list_objects( 1, null, $prefix ) ) > 0 ); } catch ( NoSuchContainerException $e ) { return false; - } catch ( InvalidResponseException $e ) { - } catch ( Exception $e ) { // some other exception? - $this->logException( $e, __METHOD__, array( 'cont' => $fullCont, 'dir' => $dir ) ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, null, __METHOD__, + array( 'cont' => $fullCont, 'dir' => $dir ) ); } return null; // error @@ -642,9 +741,9 @@ class SwiftFileBackend extends FileBackendStore { } } } catch ( NoSuchContainerException $e ) { - } catch ( InvalidResponseException $e ) { - } catch ( Exception $e ) { // some other exception? - $this->logException( $e, __METHOD__, array( 'cont' => $fullCont, 'dir' => $dir ) ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, null, __METHOD__, + array( 'cont' => $fullCont, 'dir' => $dir ) ); } return $dirs; @@ -685,9 +784,9 @@ class SwiftFileBackend extends FileBackendStore { $after = end( $files ); // update last item reset( $files ); // reset pointer } catch ( NoSuchContainerException $e ) { - } catch ( InvalidResponseException $e ) { - } catch ( Exception $e ) { // some other exception? - $this->logException( $e, __METHOD__, array( 'cont' => $fullCont, 'dir' => $dir ) ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, null, __METHOD__, + array( 'cont' => $fullCont, 'dir' => $dir ) ); } return $files; @@ -723,12 +822,8 @@ class SwiftFileBackend extends FileBackendStore { } catch ( NoSuchContainerException $e ) { $status->fatal( 'backend-fail-stream', $params['src'] ); return $status; - } catch ( InvalidResponseException $e ) { - $status->fatal( 'backend-fail-connect', $this->name ); - return $status; - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-stream', $params['src'] ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); return $status; } @@ -736,11 +831,8 @@ class SwiftFileBackend extends FileBackendStore { $output = fopen( 'php://output', 'wb' ); $obj = new CF_Object( $cont, $srcRel, false, false ); // skip HEAD request $obj->stream( $output, $this->headersFromParams( $params ) ); - } catch ( InvalidResponseException $e ) { // 404? connection problem? - $status->fatal( 'backend-fail-stream', $params['src'] ); - } catch ( Exception $e ) { // some other exception? - $status->fatal( 'backend-fail-stream', $params['src'] ); - $this->logException( $e, __METHOD__, $params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, __METHOD__, $params ); } return $status; @@ -779,11 +871,9 @@ class SwiftFileBackend extends FileBackendStore { } } catch ( NoSuchContainerException $e ) { $tmpFile = null; - } catch ( InvalidResponseException $e ) { - $tmpFile = null; - } catch ( Exception $e ) { // some other exception? + } catch ( CloudFilesException $e ) { // some other exception? $tmpFile = null; - $this->logException( $e, __METHOD__, $params ); + $this->handleException( $e, null, __METHOD__, $params ); } return $tmpFile; @@ -813,6 +903,39 @@ class SwiftFileBackend extends FileBackendStore { return $hdrs; } + /** + * @see FileBackendStore::doExecuteOpHandlesInternal() + * @return Array List of corresponding Status objects + */ + protected function doExecuteOpHandlesInternal( array $fileOpHandles ) { + $statuses = array(); + + $cfOps = array(); // list of CF_Async_Op objects + foreach ( $fileOpHandles as $index => $fileOpHandle ) { + $cfOps[$index] = $fileOpHandle->cfOp; + } + $batch = new CF_Async_Op_Batch( $cfOps ); + + $cfOps = $batch->execute(); + foreach ( $cfOps as $index => $cfOp ) { + $status = Status::newGood(); + try { // catch exceptions; update status + $function = '_getResponse' . $fileOpHandles[$index]->call; + $this->$function( $cfOp, $status, $fileOpHandles[$index]->params ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, $status, + __CLASS__ . ":$function", $fileOpHandles[$index]->params ); + } + $statuses[$index] = $status; + } + + foreach ( $fileOpHandles as $fileOpHandle ) { + $fileOpHandle->closeResources(); + } + + return $statuses; + } + /** * Set read/write permissions for a Swift container * @@ -885,6 +1008,7 @@ class SwiftFileBackend extends FileBackendStore { * @param $container string Container name * @param $bypassCache bool Bypass all caches and load from Swift * @return CF_Container + * @throws NoSuchContainerException * @throws InvalidResponseException */ protected function getContainer( $container, $bypassCache = false ) { @@ -954,31 +1078,54 @@ class SwiftFileBackend extends FileBackendStore { $info['bytes'] ); } - } catch ( InvalidResponseException $e ) { - } catch ( Exception $e ) { // some other exception? - $this->logException( $e, __METHOD__, array() ); + } catch ( CloudFilesException $e ) { // some other exception? + $this->handleException( $e, null, __METHOD__, array() ); } } /** - * Log an unexpected exception for this backend + * Log an unexpected exception for this backend. + * This also sets the Status object to have a fatal error. * * @param $e Exception + * @param $status Status|null * @param $func string * @param $params Array * @return void */ - protected function logException( Exception $e, $func, array $params ) { + protected function handleException( Exception $e, $status, $func, array $params ) { + if ( $status instanceof Status ) { + if ( $e instanceof AuthenticationException ) { + $status->fatal( 'backend-fail-connect', $this->name ); + } else { + $status->fatal( 'backend-fail-internal', $this->name ); + } + } + if ( $e->getMessage() ) { + trigger_error( "$func: " . $e->getMessage(), E_USER_WARNING ); + } wfDebugLog( 'SwiftBackend', get_class( $e ) . " in '{$func}' (given '" . FormatJson::encode( $params ) . "')" . - ( $e instanceof InvalidResponseException - ? ": {$e->getMessage()}" - : "" - ) + ( $e->getMessage() ? ": {$e->getMessage()}" : "" ) ); } } +/** + * @see FileBackendStoreOpHandle + */ +class SwiftFileOpHandle extends FileBackendStoreOpHandle { + /** @var CF_Async_Op */ + public $cfOp; + + public function __construct( $backend, array $params, $call, CF_Async_Op $cfOp ) { + $this->backend = $backend; + $this->params = $params; + $this->call = $call; + $this->cfOp = $cfOp; + } +} + /** * SwiftFileBackend helper class to page through listings. * Swift also has a listing limit of 10,000 objects for sanity. diff --git a/maintenance/fileOpPerfTest.php b/maintenance/fileOpPerfTest.php new file mode 100644 index 0000000000..b16bd95352 --- /dev/null +++ b/maintenance/fileOpPerfTest.php @@ -0,0 +1,135 @@ + 'ProfilerSimpleText' ); +error_reporting( E_ALL ); + +require_once( dirname( __FILE__ ) . '/Maintenance.php' ); + +class TestFileOpPerformance extends Maintenance { + public function __construct() { + parent::__construct(); + $this->mDescription = "Test fileop performance"; + $this->addOption( 'b1', 'Backend 1', true, true ); + $this->addOption( 'b2', 'Backend 2', false, true ); + $this->addOption( 'srcdir', 'File source directory', true, true ); + $this->addOption( 'maxfiles', 'Max files', false, true ); + } + + public function execute() { + $backend = FileBackendGroup::singleton()->get( $this->getOption( 'b1' ) ); + $this->doPerfTest( $backend ); + + if ( $this->getOption( 'b2' ) ) { + $backend = FileBackendGroup::singleton()->get( $this->getOption( 'b2' ) ); + $this->doPerfTest( $backend ); + } + + $profiler = Profiler::instance(); + $profiler->setTemplated( true ); + $profiler->logData(); // prints + } + + protected function doPerfTest( FileBackend $backend ) { + $ops1 = array(); + $ops2 = array(); + $ops3 = array(); + $ops4 = array(); + $ops5 = array(); + + $baseDir = 'mwstore://' . $backend->getName() . '/testing-cont1'; + $backend->prepare( array( 'dir' => $baseDir ) ); + + $dirname = $this->getOption( 'srcdir' ); + $dir = opendir( $dirname ); + if ( !$dir ) { + return; + } + + while ( $dir && ( $file = readdir( $dir ) ) !== false ) { + if ( $file[0] != '.' ) { + $this->output( "Using '$dirname/$file' in operations.\n" ); + $dst = $baseDir . '/' . wfBaseName( $file ); + $ops1[] = array( 'op' => 'store', 'src' => "$dirname/$file", 'dst' => $dst, 'overwrite' => 1); + $ops2[] = array( 'op' => 'copy', 'src' => "$dst", 'dst' => "$dst-1", 'overwrite' => 1); + $ops3[] = array( 'op' => 'move', 'src' => $dst, 'dst' => "$dst-2", 'overwrite' => 1); + $ops4[] = array( 'op' => 'delete', 'src' => "$dst-1", 'overwrite' => 1 ); + $ops5[] = array( 'op' => 'delete', 'src' => "$dst-2", 'overwrite' => 1 ); + } + if ( count( $ops1 ) >= $this->getOption( 'maxfiles', 20 ) ) { + break; // enough + } + } + closedir( $dir ); + $this->output( "\n" ); + + $start = microtime( true ); + $status = $backend->doOperations( $ops1, array( 'force' => 1 ) ); + $e = ( microtime( true ) - $start ) * 1000; + if ( $status->getErrorsArray() ) { + print_r( $status->getErrorsArray() ); + exit(0); + } + $this->output( $backend->getName() . ": Stored " . count( $ops1 ) . " files in $e ms.\n" ); + + $start = microtime( true ); + $backend->doOperations( $ops2, array( 'force' => 1 ) ); + $e = ( microtime( true ) - $start ) * 1000; + if ( $status->getErrorsArray() ) { + print_r( $status->getErrorsArray() ); + exit(0); + } + $this->output( $backend->getName() . ": Copied " . count( $ops2 ) . " files in $e ms.\n" ); + + $start = microtime( true ); + $backend->doOperations( $ops3, array( 'force' => 1 ) ); + $e = ( microtime( true ) - $start ) * 1000; + if ( $status->getErrorsArray() ) { + print_r( $status->getErrorsArray() ); + exit(0); + } + $this->output( $backend->getName() . ": Moved " . count( $ops3 ) . " files in $e ms.\n" ); + + $start = microtime( true ); + $backend->doOperations( $ops4, array( 'force' => 1 ) ); + $e = ( microtime( true ) - $start ) * 1000; + if ( $status->getErrorsArray() ) { + print_r( $status->getErrorsArray() ); + exit(0); + } + $this->output( $backend->getName() . ": Deleted " . count( $ops4 ) . " files in $e ms.\n" ); + + $start = microtime( true ); + $backend->doOperations( $ops5, array( 'force' => 1 ) ); + $e = ( microtime( true ) - $start ) * 1000; + if ( $status->getErrorsArray() ) { + print_r( $status->getErrorsArray() ); + exit(0); + } + $this->output( $backend->getName() . ": Deleted " . count( $ops5 ) . " files in $e ms.\n" ); + } +} + +$maintClass = "TestFileOpPerformance"; +require_once( RUN_MAINTENANCE_IF_MAIN ); diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php index 9c36a577bc..8da54e2f23 100644 --- a/tests/phpunit/includes/filerepo/FileBackendTest.php +++ b/tests/phpunit/includes/filerepo/FileBackendTest.php @@ -35,6 +35,7 @@ class FileBackendTest extends MediaWikiTestCase { $this->singleBackend = new FSFileBackend( array( 'name' => 'localtesting', 'lockManager' => 'fsLockManager', + #'parallelize' => 'implicit', 'containerPaths' => array( 'unittest-cont1' => "{$tmpPrefix}-localtesting-cont1", 'unittest-cont2' => "{$tmpPrefix}-localtesting-cont2" ) @@ -43,6 +44,7 @@ class FileBackendTest extends MediaWikiTestCase { $this->multiBackend = new FileBackendMultiWrite( array( 'name' => 'localtesting', 'lockManager' => 'fsLockManager', + 'parallelize' => 'implicit', 'backends' => array( array( 'name' => 'localmutlitesting1', @@ -220,9 +222,9 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( $op ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Store from $source to $dest succeeded without warnings ($backendName)." ); - $this->assertEquals( array(), $status->errors, + $this->assertEquals( true, $status->isOK(), "Store from $source to $dest succeeded ($backendName)." ); $this->assertEquals( array( 0 => true ), $status->success, "Store from $source to $dest has proper 'success' field in Status ($backendName)." ); @@ -297,7 +299,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); if ( isset( $op['overwrite'] ) || isset( $op['overwriteSame'] ) ) { @@ -306,7 +308,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( $op ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Copy from $source to $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Copy from $source to $dest succeeded ($backendName)." ); @@ -385,7 +387,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); if ( isset( $op['overwrite'] ) || isset( $op['overwriteSame'] ) ) { @@ -393,7 +395,7 @@ class FileBackendTest extends MediaWikiTestCase { } $status = $this->backend->doOperation( $op ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Move from $source to $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Move from $source to $dest succeeded ($backendName)." ); @@ -473,13 +475,13 @@ class FileBackendTest extends MediaWikiTestCase { if ( $withSource ) { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); } $status = $this->backend->doOperation( $op ); if ( $okStatus ) { - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Deletion of file at $source succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Deletion of file at $source succeeded ($backendName)." ); @@ -555,13 +557,13 @@ class FileBackendTest extends MediaWikiTestCase { if ( $alreadyExists ) { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $oldText, 'dst' => $dest ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $dest succeeded ($backendName)." ); } $status = $this->backend->doOperation( $op ); if ( $okStatus ) { - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Creation of file at $dest succeeded ($backendName)." ); @@ -685,7 +687,7 @@ class FileBackendTest extends MediaWikiTestCase { } $status = $this->backend->doOperations( $ops ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of source files succeeded ($backendName)." ); $dest = $params['dst']; @@ -702,7 +704,7 @@ class FileBackendTest extends MediaWikiTestCase { // Combine the files into one $status = $this->backend->concatenate( $params ); if ( $okStatus ) { - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of concat file at $dest succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Creation of concat file at $dest succeeded ($backendName)." ); @@ -802,7 +804,7 @@ class FileBackendTest extends MediaWikiTestCase { if ( $alreadyExists ) { $this->prepare( array( 'dir' => dirname( $path ) ) ); $status = $this->backend->create( array( 'dst' => $path, 'content' => $content ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $path succeeded ($backendName)." ); $size = $this->backend->getFileSize( array( 'src' => $path ) ); @@ -864,7 +866,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $content, 'dst' => $source ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Creation of file at $source succeeded with OK status ($backendName)." ); @@ -909,7 +911,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $content, 'dst' => $source ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); $tmpFile = $this->backend->getLocalCopy( array( 'src' => $source ) ); @@ -952,7 +954,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->doOperation( array( 'op' => 'create', 'content' => $content, 'dst' => $source ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of file at $source succeeded ($backendName)." ); $tmpFile = $this->backend->getLocalReference( array( 'src' => $source ) ); @@ -1001,7 +1003,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->prepare( array( 'dir' => dirname( $path ) ) ); if ( $isOK ) { - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Preparing dir $path succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Preparing dir $path succeeded ($backendName)." ); @@ -1012,7 +1014,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->clean( array( 'dir' => dirname( $path ) ) ); if ( $isOK ) { - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Cleaning dir $path succeeded without warnings ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Cleaning dir $path succeeded ($backendName)." ); @@ -1052,7 +1054,7 @@ class FileBackendTest extends MediaWikiTestCase { ); foreach ( $dirs as $dir ) { $status = $this->prepare( array( 'dir' => $dir ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Preparing dir $dir succeeded without warnings ($backendName)." ); } @@ -1065,7 +1067,7 @@ class FileBackendTest extends MediaWikiTestCase { $status = $this->backend->clean( array( 'dir' => "$base/unittest-cont1", 'recursive' => 1 ) ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Recursive cleaning of dir $dir succeeded without warnings ($backendName)." ); foreach ( $dirs as $dir ) { @@ -1089,15 +1091,23 @@ class FileBackendTest extends MediaWikiTestCase { $this->backend = $this->singleBackend; $this->tearDownFiles(); - $this->doTestDoOperationsFailing(); + $this->doTestDoOperations2(); $this->tearDownFiles(); $this->backend = $this->multiBackend; $this->tearDownFiles(); + $this->doTestDoOperations2(); + $this->tearDownFiles(); + + $this->backend = $this->singleBackend; + $this->tearDownFiles(); $this->doTestDoOperationsFailing(); $this->tearDownFiles(); - // @TODO: test some cases where the ops should fail + $this->backend = $this->multiBackend; + $this->tearDownFiles(); + $this->doTestDoOperationsFailing(); + $this->tearDownFiles(); } private function doTestDoOperations() { @@ -1148,7 +1158,7 @@ class FileBackendTest extends MediaWikiTestCase { // Does nothing ) ); - $this->assertEquals( array(), $status->errors, "Operation batch succeeded" ); + $this->assertGoodStatus( $status, "Operation batch succeeded" ); $this->assertEquals( true, $status->isOK(), "Operation batch succeeded" ); $this->assertEquals( 13, count( $status->success ), "Operation batch has correct success array" ); @@ -1173,7 +1183,94 @@ class FileBackendTest extends MediaWikiTestCase { "Correct file SHA-1 of $fileC" ); } - private function doTestDoOperationsFailing() { + // concurrency orientated + function doTestDoOperations2() { + $base = $this->baseStorePath(); + + $fileAContents = '3tqtmoeatmn4wg4qe-mg3qt3 tq'; + $fileBContents = 'g-jmq3gpqgt3qtg q3GT '; + $fileCContents = 'eigna[ogmewt 3qt g3qg flew[ag'; + + $tmpNameA = TempFSFile::factory( "unittests_", 'txt' )->getPath(); + file_put_contents( $tmpNameA, $fileAContents ); + $tmpNameB = TempFSFile::factory( "unittests_", 'txt' )->getPath(); + file_put_contents( $tmpNameB, $fileBContents ); + $tmpNameC = TempFSFile::factory( "unittests_", 'txt' )->getPath(); + file_put_contents( $tmpNameC, $fileCContents ); + + $this->filesToPrune[] = $tmpNameA; # avoid file leaking + $this->filesToPrune[] = $tmpNameB; # avoid file leaking + $this->filesToPrune[] = $tmpNameC; # avoid file leaking + + $fileA = "$base/unittest-cont1/a/b/fileA.txt"; + $fileB = "$base/unittest-cont1/a/b/fileB.txt"; + $fileC = "$base/unittest-cont1/a/b/fileC.txt"; + $fileD = "$base/unittest-cont1/a/b/fileD.txt"; + + $this->prepare( array( 'dir' => dirname( $fileA ) ) ); + $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) ); + $this->prepare( array( 'dir' => dirname( $fileB ) ) ); + $this->prepare( array( 'dir' => dirname( $fileC ) ) ); + $this->prepare( array( 'dir' => dirname( $fileD ) ) ); + + $status = $this->backend->doOperations( array( + array( 'op' => 'store', 'src' => $tmpNameA, 'dst' => $fileA, 'overwriteSame' => 1 ), + array( 'op' => 'store', 'src' => $tmpNameB, 'dst' => $fileB, 'overwrite' => 1 ), + array( 'op' => 'store', 'src' => $tmpNameC, 'dst' => $fileC, 'overwrite' => 1 ), + array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC, 'overwrite' => 1 ), + // Now: A:, B:, C:, D: (file:) + array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileA, 'overwriteSame' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileD, 'overwrite' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'move', 'src' => $fileB, 'dst' => $fileC ), + // Now: A:, B:, C:, D: + array( 'op' => 'move', 'src' => $fileD, 'dst' => $fileA, 'overwriteSame' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileA, 'overwrite' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC ), + // Now: A:, B:, C:, D: + array( 'op' => 'move', 'src' => $fileA, 'dst' => $fileC, 'overwriteSame' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileC, 'overwrite' => 1 ), + // Does nothing + array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileC, 'overwriteSame' => 1 ), + // Does nothing + array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileC, 'overwrite' => 1 ), + // Does nothing + array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileC, 'overwriteSame' => 1 ), + // Does nothing + array( 'op' => 'null' ), + // Does nothing + ) ); + + $this->assertGoodStatus( $status, "Operation batch succeeded" ); + $this->assertEquals( true, $status->isOK(), "Operation batch succeeded" ); + $this->assertEquals( 16, count( $status->success ), + "Operation batch has correct success array" ); + + $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileA ) ), + "File does not exist at $fileA" ); + $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileB ) ), + "File does not exist at $fileB" ); + $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileD ) ), + "File does not exist at $fileD" ); + + $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $fileC ) ), + "File exists at $fileC" ); + $this->assertEquals( $fileBContents, + $this->backend->getFileContents( array( 'src' => $fileC ) ), + "Correct file contents of $fileC" ); + $this->assertEquals( strlen( $fileBContents ), + $this->backend->getFileSize( array( 'src' => $fileC ) ), + "Correct file size of $fileC" ); + $this->assertEquals( wfBaseConvert( sha1( $fileBContents ), 16, 36, 31 ), + $this->backend->getFileSha1Base36( array( 'src' => $fileC ) ), + "Correct file SHA-1 of $fileC" ); + } + + function doTestDoOperationsFailing() { $base = $this->baseStorePath(); $fileA = "$base/unittest-cont2/a/b/fileA.txt"; @@ -1275,7 +1372,7 @@ class FileBackendTest extends MediaWikiTestCase { $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file ); } $status = $this->backend->doOperations( $ops ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of files succeeded ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Creation of files succeeded with OK status ($backendName)." ); @@ -1428,7 +1525,7 @@ class FileBackendTest extends MediaWikiTestCase { $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file ); } $status = $this->backend->doOperations( $ops ); - $this->assertEquals( array(), $status->errors, + $this->assertGoodStatus( $status, "Creation of files succeeded ($backendName)." ); $this->assertEquals( true, $status->isOK(), "Creation of files succeeded with OK status ($backendName)." ); @@ -1654,11 +1751,11 @@ class FileBackendTest extends MediaWikiTestCase { } private function recursiveClean( $dir ) { - do { - if ( !$this->backend->clean( array( 'dir' => $dir ) )->isOK() ) { - break; - } - } while ( $dir = FileBackend::parentStoragePath( $dir ) ); + $this->backend->clean( array( 'dir' => $dir, 'recursive' => 1 ) ); + } + + function assertGoodStatus( $status, $msg ) { + $this->assertEquals( print_r( array(), 1 ), print_r( $status->errors, 1 ), $msg ); } function tearDown() { -- 2.20.1